Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Refactor] Extend REPL to support external metadata storage and data storage #381

Merged
merged 7 commits into from
Aug 28, 2024
Merged

[Refactor] Extend REPL to support external metadata storage and data storage #381

merged 7 commits into from
Aug 28, 2024

Conversation

noCharger
Copy link
Collaborator

@noCharger noCharger commented Jun 12, 2024 •
edited
Loading

Description

This is the last PR of [RFC] Flint main class refactor

  1. Introduce new spark conf for REPL to support external metadata storage and data storage access
spark.flint.job.customSessionManager
spark.flint.job.customStatementManager
spark.flint.job.customQueryResultWriter
  1. Add new interfaces with default implementation which uses original opensearch reader and writer
flint-commons/src/main/scala/org/apache/spark/sql/SessionManager.scala
flint-commons/src/main/scala/org/apache/spark/sql/StatementsExecutionManager.scala
flint-commons/src/main/scala/org/apache/spark/sql/QueryResultWriter.scala

spark-sql-application/src/main/scala/org/apache/spark/sql/SessionManagerImpl.scala
spark-sql-application/src/main/scala/org/apache/spark/sql/StatementsExecutionManagerImpl.scala
spark-sql-application/src/main/scala/org/apache/spark/sql/QueryResultWriterImpl.scala
  1. Decouple OpenSearch specific logics from FlintREPL
  • FlintReader for reading request metadata (statement doc), now is moved to StatementsExecutionManagerImpl
  • osClient for reading request metadata (session doc), now is moved to SessionManagerImpl
  • osClient for writing result data, now is moved to QueryResultWriterImpl
  • OpenSearchUpdater for write request metadata (session doc and statement doc), now is moved to SessionManagerImpl and StatementsExecutionManagerImpl
  1. Make resultIndex as optional for FlintREPL, to support external data storage

Issues Resolved

#376

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@noCharger noCharger force-pushed the repl-refactor branch 12 times, most recently from 3cc87db to 22965c0 Compare June 20, 2024 07:01
@noCharger noCharger force-pushed the repl-refactor branch 5 times, most recently from 84312d5 to 35ed065 Compare June 21, 2024 23:45
seankao-az
seankao-az reviewed Jun 22, 2024
View reviewed changes
flint-commons/src/main/scala/org/opensearch/flint/data/InteractiveSession.scala Outdated Show resolved Hide resolved
flint-commons/src/main/scala/org/opensearch/flint/data/InteractiveSession.scala Outdated Show resolved Hide resolved
spark-sql-application/src/main/scala/org/apache/spark/sql/FlintJobExecutor.scala Outdated Show resolved Hide resolved
spark-sql-application/src/main/scala/org/apache/spark/sql/FlintREPL.scala Outdated Show resolved Hide resolved
spark-sql-application/src/main/scala/org/apache/spark/sql/InMemoryQueryExecutionState.scala Outdated Show resolved Hide resolved
spark-sql-application/src/main/scala/org/apache/spark/sql/SessionManagerImpl.scala Outdated Show resolved Hide resolved
flint-spark-integration/src/main/scala/org/apache/spark/sql/flint/config/FlintSparkConf.scala Outdated Show resolved Hide resolved
spark-sql-application/src/main/scala/org/apache/spark/sql/SessionManagerImpl.scala Outdated Show resolved Hide resolved
kaituo
kaituo reviewed Jun 25, 2024
View reviewed changes
Copy link
Collaborator

@kaituo kaituo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides CI, what manual tests have you done? Have you done the following two:
First, can we stop a job when another job exclude it using exclude id?
Second, if a query takes too long, will we can cancel the query?

spark-sql-application/src/main/scala/org/apache/spark/sql/FlintJob.scala Outdated Show resolved Hide resolved
@noCharger noCharger mentioned this pull request Aug 10, 2024
Closed
@noCharger noCharger force-pushed the repl-refactor branch 3 times, most recently from ff41d16 to 1b277ad Compare August 15, 2024 15:43
@noCharger noCharger self-assigned this Aug 15, 2024
@noCharger noCharger added 0.5.1 enhancement New feature or request labels Aug 15, 2024
@noCharger noCharger changed the title REPL refactor [Refactor] Extend REPL to support external metadata storage and data storage Aug 16, 2024
@noCharger noCharger marked this pull request as ready for review August 16, 2024 03:33
@noCharger
Clean up logs and add UTs
93f2e71 
Signed-off-by: Louis Chu <[email protected]>
@noCharger
Clean up and fix UTs
28bacdb 
Signed-off-by: Louis Chu <[email protected]>
@noCharger noCharger merged commit aa509ac into opensearch-project:main Aug 28, 2024
5 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 28, 2024
@github-actions
[Refactor] Extend REPL to support external metadata storage and data …
0c2df5b 
…storage (#381)

* REPL refactor

Signed-off-by: Louis Chu <[email protected]>

* Ignore UT

Signed-off-by: Louis Chu <[email protected]>

* Add seqNo and primaryTerm to session object

Signed-off-by: Louis Chu <[email protected]>

* Address os client concurrency limitation

Signed-off-by: Louis Chu <[email protected]>

* Add interface for query execution

Signed-off-by: Louis Chu <[email protected]>

* Clean up logs and add UTs

Signed-off-by: Louis Chu <[email protected]>

* Clean up and fix UTs

Signed-off-by: Louis Chu <[email protected]>

---------

Signed-off-by: Louis Chu <[email protected]>
(cherry picked from commit aa509ac)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Aug 28, 2024
Merged
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 28, 2024
@github-actions
[Refactor] Extend REPL to support external metadata storage and data …
ec9960b 
…storage (#381)

* REPL refactor

Signed-off-by: Louis Chu <[email protected]>

* Ignore UT

Signed-off-by: Louis Chu <[email protected]>

* Add seqNo and primaryTerm to session object

Signed-off-by: Louis Chu <[email protected]>

* Address os client concurrency limitation

Signed-off-by: Louis Chu <[email protected]>

* Add interface for query execution

Signed-off-by: Louis Chu <[email protected]>

* Clean up logs and add UTs

Signed-off-by: Louis Chu <[email protected]>

* Clean up and fix UTs

Signed-off-by: Louis Chu <[email protected]>

---------

Signed-off-by: Louis Chu <[email protected]>
(cherry picked from commit aa509ac)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Aug 28, 2024
Merged
noCharger pushed a commit that referenced this pull request Aug 28, 2024
@opensearch-trigger-bot @github-actions
[Refactor] Extend REPL to support external metadata storage and data …
ad5697b 
…storage (#381) (#605)

* REPL refactor



* Ignore UT



* Add seqNo and primaryTerm to session object



* Address os client concurrency limitation



* Add interface for query execution



* Clean up logs and add UTs



* Clean up and fix UTs



---------


(cherry picked from commit aa509ac)

Signed-off-by: Louis Chu <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
noCharger pushed a commit that referenced this pull request Sep 8, 2024
@opensearch-trigger-bot @github-actions
[Refactor] Extend REPL to support external metadata storage and data …
d5b87f3 
…storage (#381) (#604)

* REPL refactor



* Ignore UT



* Add seqNo and primaryTerm to session object



* Address os client concurrency limitation



* Add interface for query execution



* Clean up logs and add UTs



* Clean up and fix UTs



---------


(cherry picked from commit aa509ac)

Signed-off-by: Louis Chu <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@noCharger noCharger mentioned this pull request Nov 5, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seankao-az seankao-az seankao-az approved these changes

@kaituo kaituo kaituo approved these changes

@dai-chen dai-chen Awaiting requested review from dai-chen dai-chen is a code owner

@rupal-bq rupal-bq Awaiting requested review from rupal-bq

@vmmusings vmmusings Awaiting requested review from vmmusings

@penghuo penghuo Awaiting requested review from penghuo penghuo is a code owner

@anirudha anirudha Awaiting requested review from anirudha anirudha is a code owner

@YANG-DB YANG-DB Awaiting requested review from YANG-DB YANG-DB is a code owner

Assignees

@noCharger noCharger

Labels
0.5.1 backport 0.5-nexus backport 0.5 enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@noCharger @kaituo @seankao-az